-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
3461 bootstrap 5 upgrade #3541
3461 bootstrap 5 upgrade #3541
Conversation
0e2bb88
to
031742e
Compare
@johrstrom @Oglopf Can you pull this down when you have a moment and check if the action buttons on the Files Browser page have outlines? I cannot for the life of me figure out why some |
Sure, it's due to the ondemand/apps/dashboard/app/assets/stylesheets/_variables.scss Lines 13 to 18 in 75ea435
When we SASS compile - bootstrap will make a And again, we don't have I don't know why we've set that variable. I'd have to look into the git history, and even then it may not be mentioned why we did so, but it seems that after we redefine some colors, we should likely just add the defaults to that list. https://getbootstrap.com/docs/5.0/customize/color/#theme-colors |
031742e
to
4328885
Compare
@@ -7,14 +7,14 @@ | |||
<% end %> | |||
|
|||
<%- if Configuration.bc_saved_settings? -%> | |||
<div class="form-group form-check"> | |||
<div class="mb3 form-check"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be mb-3
?
<div class="form-group d-flex"> | ||
<input class="form-control w-50 mr-2" name="template_name" value="<%= @template_name %>" type="text" id="batch_connect_session_template_name" aria-label="<%= t('dashboard.batch_connect_form_template_name_label') %>" readonly> | ||
<%= f.submit t('dashboard.batch_connect_form_save_submit'), id: 'batch_connect_session_save_template_submit', class: "ml-auto btn btn-primary", formaction: batch_connect_save_settings_path(token: @app.token), disabled: @template_name ? false : true %> | ||
<div class="mb3 d-flex"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same mb-3
typo here.
This looks fine and works well. I just found 2 typos that we may as well fix now because they may be overlooked if we defer it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Though we should keep #3461 open and make tickets under it when we find issues.
Fixes #3461
Bootstrap 5 Upgrade
Builds & tests should all pass. Each screen in this branch should look the same as its counterpart in ondemand-test.